Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQLAlchemy 2.0 compatibility #901

Merged
merged 5 commits into from
Aug 25, 2023
Merged

SQLAlchemy 2.0 compatibility #901

merged 5 commits into from
Aug 25, 2023

Conversation

mayty
Copy link
Contributor

@mayty mayty commented Aug 24, 2023

What do these changes do?

Fix SQLAlchemy 2.0 compatibility

Are there changes in behavior for the user?

No

Related issue number

897.bugfix

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> (e.g. 588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the PR
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: Fix issue with non-ascii contents in doctest text files.

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Merging #901 (705faac) into master (0b5e63c) will increase coverage by 1.32%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #901      +/-   ##
==========================================
+ Coverage   91.77%   93.09%   +1.32%     
==========================================
  Files           9       12       +3     
  Lines         863     1579     +716     
  Branches      118      188      +70     
==========================================
+ Hits          792     1470     +678     
- Misses         53       77      +24     
- Partials       18       32      +14     
Files Changed Coverage Δ
aiopg/sa/engine.py 99.17% <100.00%> (+0.03%) ⬆️
aiopg/sa/result.py 90.47% <100.00%> (ø)

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Pliner
Copy link
Member

Pliner commented Aug 24, 2023

Hey @mayty,

Thank you for the contribution.

I have pushed CI changes (not ideal through) to run tests on both major versions of sqlalchemy.

@Pliner
Copy link
Member

Pliner commented Aug 24, 2023

The changes look reasonable for me, so I am happy to merge and release a new beta version.

@mayty Do you want to include any other additional changes into this PR?

@Pliner Pliner marked this pull request as ready for review August 24, 2023 20:39
@mayty
Copy link
Contributor Author

mayty commented Aug 24, 2023

Hi
That's all the changes I want to include

But older version sqlalchemy test should probably be limited to >=1.4,<1.5. They would fail on 1.3 due to new syntax

@Pliner
Copy link
Member

Pliner commented Aug 24, 2023

But older version sqlalchemy test should probably be limited to >=1.4,<1.5. They would fail on 1.3 due to new syntax

Thanks for pointing this out. I have bumped a min supported sqlalchemy version to 1.4.

@Pliner
Copy link
Member

Pliner commented Aug 24, 2023

@mayty I was thinking to release it as 1.5 with a minor version bump. What do you think?

@mayty
Copy link
Contributor Author

mayty commented Aug 24, 2023

I agree

@Pliner Pliner merged commit 653ba5a into aio-libs:master Aug 25, 2023
14 checks passed
@Pliner
Copy link
Member

Pliner commented Aug 29, 2023

https://pypi.org/project/aiopg/1.5.0a1/

@Pliner Pliner mentioned this pull request Aug 30, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants